Skip to content

test(playwright): un-skip ep_headings2 spec under WITH_PLUGINS (#7626)#7634

Merged
JohnMcLear merged 1 commit intoether:developfrom
JohnMcLear:fix/unskip-ep-headings2-with-plugins-7626
Apr 30, 2026
Merged

test(playwright): un-skip ep_headings2 spec under WITH_PLUGINS (#7626)#7634
JohnMcLear merged 1 commit intoether:developfrom
JohnMcLear:fix/unskip-ep-headings2-with-plugins-7626

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

Risk

Lowest possible: one config entry removed, no test or production code touched. If the spec is still flaky under WITH_PLUGINS, CI fails on this PR (firefox + WITH_PLUGINS retries 5×) and we put the ignore back with a narrower comment about what specifically still races. No release artefacts affected.

Semver

patch — test infrastructure only.

Test plan

  • Playwright Firefox with plugins job passes on this PR
  • Playwright Chrome with plugins job passes (sanity check — wasn't ignored there but worth confirming)
  • ep_headings2 .../headings.spec.ts:44 Option select is changed when heading is changed line appears in the firefox-with-plugins log as (not skipped, not failed)

🤖 Generated with Claude Code

Remove the FRONTEND_IGNORE entry that suppressed
ep_headings2/static/tests/frontend-new/specs/headings.spec.ts under
WITH_PLUGINS=1. The skip was added in ether#7628 while the keystroke-drop
flake (ether#7611) was still being chased; ether#7630 then identified the actual
root cause as ep_cursortrace's per-keystroke cursorPosition socket
spam saturating Firefox's input pipeline, removed ep_cursortrace
from the WITH_PLUGINS plugin set, and added waitForEditorReady() to
goToNewPad/goToPad. With both root causes addressed, this skip is
likely stale — the spec's own "Option select is changed when heading
is changed" test already uses insertText for the second-line typing,
so it should clear the same bar that ether#7630 cleared for ep_markdown
and ep_spellcheck (both now passing on develop).

Closes ether#7626 if CI confirms — the issue's three plugin
specs (markdown, spellcheck, headings2) and timeslider_identity_changeset
are all addressed once this lands. If headings2 is still flaky after
this, FRONTEND_IGNORE comes back with a narrower comment about what
specifically still races.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Un-skip ep_headings2 spec under WITH_PLUGINS configuration

🧪 Tests

Grey Divider

Walkthroughs

Description
• Remove ep_headings2 spec from FRONTEND_IGNORE under WITH_PLUGINS
• Root causes of keystroke-drop flake already addressed in #7630
• Spec uses insertText for typing, clearing same race condition
• Config-only change, no test or production code modified
Diagram
flowchart LR
  A["ep_headings2 spec<br/>previously ignored"] -- "remove from<br/>FRONTEND_IGNORE" --> B["spec now runs<br/>under WITH_PLUGINS"]
  C["Root causes fixed<br/>in #7630"] -- "ep_cursortrace removed<br/>waitForEditorReady added" --> B
  B -- "insertText usage<br/>prevents races" --> D["spec passes<br/>on Firefox+plugins"]
Loading

Grey Divider

File Changes

1. src/playwright.config.ts ⚙️ Configuration changes +1/-8

Remove ep_headings2 from Playwright ignore list

• Removed conditional FRONTEND_IGNORE logic that skipped ep_headings2 spec under WITH_PLUGINS
• Simplified FRONTEND_IGNORE to empty array constant
• Deleted explanatory comment about keystroke-drop flake and #7611 tracking
• Change enables ep_headings2 spec to run in WITH_PLUGINS test configuration

src/playwright.config.ts


Grey Divider

Qodo Logo

@JohnMcLear
Copy link
Copy Markdown
Member Author

/review

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 30, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used

Grey Divider


Advisory comments

1. Lost flake rationale 🐞 Bug ⚙ Maintainability
Description
The PR removes the WITH_PLUGINS-only ignore and also deletes the inline comments (including the
tracking reference) that explained why the workaround existed. If the ep_headings2 spec becomes
flaky again, it will be harder to quickly understand/reapply the prior decision without
re-investigating history.
Code

src/playwright.config.ts[23]

+const FRONTEND_IGNORE: string[] = [];
Evidence
FRONTEND_IGNORE is now an unconditional empty list with no surrounding context, whereas the removed
block previously documented the flake/workaround rationale and referenced the tracking issue.
Keeping a short note (even after un-skipping) preserves institutional knowledge for future
regressions.

src/playwright.config.ts[21-24]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The previous WITH_PLUGINS-only ignore block also contained the rationale and tracking reference for the flake workaround; replacing it with an always-empty ignore list loses that context.

### Issue Context
Even though the ignore is intentionally removed to re-enable coverage, a short comment noting the historical flake and (optionally) why it was removed helps future maintainers if the test flakes again.

### Fix Focus Areas
- src/playwright.config.ts[21-24]

### Suggested fix
Add a short comment above `FRONTEND_IGNORE` that references the historical flake/tracking issue and (optionally) the PR(s) that resolved it, while keeping `FRONTEND_IGNORE` empty.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Apr 30, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@JohnMcLear JohnMcLear merged commit cafd60a into ether:develop Apr 30, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Playwright Firefox-with-plugins: 4 specs fail with non-keystroke root causes after #7622

1 participant